Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 65-66: The populated bookedBy user documents are exposing full
user data; update both populate calls (the one using
RoomBooking.find(...).populate('room event bookedBy') and the similar populate
at lines ~121-122) to restrict fields by using the object form: populate({ path:
'bookedBy', select: '_id name email' }) (or whichever minimal safe fields your
User model exposes), e.g., replace the string 'bookedBy' with the object
selector so only safe user fields are returned while keeping room and event
populations unchanged.
- Around line 19-23: getAllRooms currently returns only static Room docs from
Room.find and must be extended to include live status fields (status,
current_event, occupied_until). After fetching rooms in getAllRooms, retrieve
live occupancy data (from your RoomStatus/Occupancy store or service) for each
room (e.g., via RoomStatus.findOne({ roomId }) or
RoomStatusService.getStatus(room.id)), merge those fields into each room object,
and then send the augmented array with res.json; perform the per-room lookups
concurrently with Promise.all to avoid serial waits and handle missing status
records by supplying sensible defaults.
- Around line 31-39: The clash logic in the RoomBooking.findOne query ignores
the booking date and doesn’t validate time ranges; update the controller to
first validate that startTime < endTime (reject requests where startTime >=
endTime) and then include the booking date in the query (e.g., filter by date or
by combining date+time) so only bookings on the same date are checked; keep the
existing room: roomId and status: { $in: ['Pending','Approved'] } filters and
use the same overlap condition ({ startTime: { $lt: endTime }, endTime: { $gt:
startTime } }) but applied together with date to prevent cross-day matches.
- Around line 77-84: Before updating to 'Approved', load the target booking (use
RoomBooking.findById(id)) to get its room and time window, then if status ===
'Approved' run a query for any other booking with status 'Approved' for the same
room that overlaps the target booking's start/end (e.g. findOne({ _id: { $ne: id
}, room: booking.room, status: 'Approved', $or: [{ start: { $lt: booking.end,
$gte: booking.start } }, { end: { $gt: booking.start, $lte: booking.end } }, {
start: { $lte: booking.start }, end: { $gte: booking.end } }] }) ) and if one
exists return a 409/conflict; only then perform the update
(RoomBooking.findByIdAndUpdate or findOneAndUpdate). Ensure you reference
RoomBooking.findById and RoomBooking.findByIdAndUpdate in the change.

In `@backend/models/schema.js`:
- Around line 648-666: The roomSchema is missing the API-required fields room_id
and allowed_roles; update the mongoose roomSchema to add a room_id field
(String, required, unique — generated if absent via a default function or a
pre('save') hook using UUID logic) and an allowed_roles field (Array of String,
required or with a sensible default like []), and ensure any creation/update
code that constructs Room documents (e.g., create handlers or Model.create
usages) no longer omit room_id so responses match the API contract; keep the
symbol name roomSchema and adjust indexes/validation as needed.

In `@backend/routes/roomBooking.js`:
- Around line 11-13: The route paths are incorrect: change router.get('/rooms',
isAuthenticated, roomBookingController.getAllRooms) to router.get('/',
isAuthenticated, roomBookingController.getAllRooms) so the mounted /api/rooms
yields GET /api/rooms; add a new parameterized route router.get('/:room_id',
isAuthenticated, roomBookingController.getRoomById) (or the appropriate
controller method that returns room details/status) to expose GET
/api/rooms/:room_id; also update the other similar occurrence (the one around
line 35) to use '/' or '/:room_id' instead of duplicating "rooms". Ensure the
referenced controller method name matches an exported function in
roomBookingController.

In `@backend/seed.js`:
- Around line 26-31: The seedRooms function is defined but never invoked, so
sampleRooms are not inserted; call seedRooms from the main seeding flow (for
example inside the top-level async seed/run function or after other seed calls)
or add it to the Promise.all/await chain that runs other seed helpers so it
executes during seeding; locate the seedRooms function and ensure the script
invokes seedRooms() (or exports and calls it where other seeds like
seedUsers/seedBookings are executed) so rooms get inserted on a fresh seed.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 190-198: The handler currently returns the flat room document
enriched with occupancy fields (via Room.findById, SAFE_ROOM_SELECT,
buildRoomStatusMap, enrichRoom) but must return the room-detail shape required
by GET /room/:room_id: wrap the response into the detail payload by constructing
schedule: { start, end } from the occupancy/statusMap data and metadata: {
capacity, features } from the room document (instead of the flat keys), then
return res.json(detailPayload); update the code around Room.findById/enrichRoom
to build and return this detailPayload object (preserving existing values but
reshaping to the detail contract).
- Around line 62-89: buildRoomStatusMap currently only queries RoomBooking and
ignores active Event-only occupancies; update it to also query the Event model
for events whose startTime <= now < endTime for the given roomIds (use the same
SAFE_EVENT_SELECT projection) and merge those results into the returned
statusMap so rooms with active events (even without a RoomBooking) are marked
status: "occupied" with current_event and occupied_until populated; ensure you
reference the existing variables/function name buildRoomStatusMap,
SAFE_EVENT_SELECT, and RoomBooking when implementing the additional Event query
and merge logic.
- Around line 91-97: The code returns the undocumented status string "available"
for empty rooms; update enrichRoom (function enrichRoom) to return "vacant"
instead of "available" in its fallback status and similarly change the
create-room response logic (the create-room response branch that currently emits
"available") to emit "vacant" so the API matches the documented enum ("vacant" |
"occupied"); ensure the three fallback fields (status, current_event,
occupied_until) keep the same values but with status set to "vacant".
- Around line 276-303: The current read-then-write flow (RoomBooking.findOne ...
then new RoomBooking ... booking.save) is TOCTOU-prone; wrap the check+insert in
a MongoDB transaction (start a session, run RoomBooking.findOne with { session
}, abort if clash, then insert/save the new RoomBooking with the same session
and commit) so the check and write are atomic and serialized; apply the same
transactional pattern to the other create/approve flow that also uses
RoomBooking.findOne and booking.save (the second block referenced in the
comment) so concurrent requests cannot both pass the check and double-book a
room.
- Around line 371-375: After retrieving the booking with
RoomBooking.findById(id) in the roomBookingController handler, restrict further
review/approval to only bookings with status "Pending": check booking.status (or
the enum used) and if it's not "Pending" return an error response (e.g., 400/409
with a clear message like "Only pending bookings can be reviewed") so
Cancelled/Rejected/Completed bookings cannot be resurrected; update any related
logic that assumes reviewable bookings and add/adjust tests to cover non-pending
status rejection.

In `@backend/routes/roomBooking.js`:
- Around line 22-27: The POST /bookings route is incorrectly blocked by the
authorizeRole(ROLE_GROUPS.ADMIN) middleware so non-admins cannot submit booking
requests; remove the authorizeRole(ROLE_GROUPS.ADMIN) middleware from the
router.post that registers roomBookingController.bookRoom (leave isAuthenticated
in place) so the controller's own room.allowed_roles and bookedBy logic can
enforce permissions and create pending requests; apply the same removal for the
other booking-creation route that uses authorizeRole(ROLE_GROUPS.ADMIN) (the
second occurrence mentioned) while keeping admin-only middleware on routes that
truly require admin privileges.

---

Nitpick comments:
In `@backend/seed.js`:
- Line 1147: The script calls seedRooms() twice causing rooms to be recreated
and orphaning room-linked data; remove the second invocation (the await
seedRooms(); near the end of the file) so rooms are only seeded once, and verify
no other duplicate calls to seedRooms remain (ensure RoomBooking or other
room-referencing seeding runs after the single seedRooms() call).

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/models/passportConfig.js`:
- Around line 68-75: In passport.deserializeUser's async callback (the function
passed to passport.deserializeUser) explicitly handle the case where
User.findById(id) returns null: after awaiting User.findById(id) check if user
is null and call done(null, false) (and optionally log a warning) instead of
passing null user into done; leave the try/catch for DB errors unchanged so
errors still call done(err, null).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 357-363: Clash detection queries currently only check RoomBooking
(e.g., the RoomBooking.findOne that sets clash) but buildRoomStatusMap treats
Event as occupancy too, so modify the overlap checks (both the initial
create/check block around the clash variable and the approval flow after loading
the booking's room/location) to also query the Event collection for overlapping
events in the same room/time window and treat them as conflicts; when checking
during approval, exclude the Event linked to the current RoomBooking (if any) to
avoid self-conflict. Ensure you mirror the same time/room filtering and the
exclusion of a linked event in both places so bookings cannot be
created/approved over event-only usages.
- Around line 94-105: The venue->room mapping only indexes room.name and
room.room_id but must also index room.location (use the room.location field) so
lookups using Event.schedule.venue will resolve correctly; also when computing
live occupancy exclude cancelled events by adding a filter on the Event query
(or schedule entry) to skip events with a cancelled status/flag (e.g.,
Event.status === 'cancelled' or schedule.cancelled) before counting overlaps.
Update the block that builds venueToRoomId (referencing Room.find and
venueToRoomId) to set venueToRoomId.set(room.location, String(room._id)) and
update the occupancy query code that counts overlapping schedules to filter out
cancelled events (the same change should be applied to the similar mapping/usage
later in the file).

---

Nitpick comments:
In `@backend/routes/roomBooking.js`:
- Around line 51-74: The duplicated role array passed into authorizeRole for
router.patch and router.put should be replaced with a single exported constant
(e.g., REVIEW_ROLES or reuse the controller's ADMIN_ROLES) to prevent drift;
define and export that shared constant from a central module (or the controller
that already defines ADMIN_ROLES), import it where the routes are declared, and
pass it into authorizeRole in both router.patch and router.put instead of the
inline array (affecting router.patch, router.put, authorizeRole, and
roomBookingController.updateBookingStatus references).

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/controllers/roomBookingController.js`:
- Around line 651-686: The cancelBooking handler currently sets booking.status =
"Cancelled" unconditionally; add a guard like in updateBookingStatus to reject
attempts to cancel when booking.status is already "Cancelled" or "Rejected"
(check booking.status before mutating), returning a 400/appropriate response
(e.g., res.status(400).json({ message: "Booking already cancelled or rejected"
})); keep existing authorization logic (ADMIN_ROLES, req.user._id) and only
update updated_at and save when the status transition is allowed, then continue
to populate and return responseBooking as before.
- Around line 5-12: The ADMIN_ROLES array in roomBookingController.js duplicates
ROLE_GROUPS.ADMIN from roles.js; import ROLE_GROUPS (or ROLE_GROUPS.ADMIN) from
roles.js and replace the local hard-coded ADMIN_ROLES definition by referencing
ROLE_GROUPS.ADMIN (or remove ADMIN_ROLES and use ROLE_GROUPS.ADMIN in its call
sites), ensuring any existing uses (e.g., permission checks in
roomBookingController) still refer to the imported group to avoid duplication
and maintenance drift.